Skip to content

Fix #26233: Combination of pickup measure and linked staff makes scor… #27694

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

skalra4
Copy link

@skalra4 skalra4 commented Apr 14, 2025

…e corrupted

Resolves: #26233

Case for pickup measures with linked staves. If you switch the order of ascending rests, there may be an empty segment due to the longer rest/note, which was not being copied to the linked staff, which when created used the default staff configuration, causing a rest or note to be left behind, seemingly "creating a duplicate" and corrupting the score.

  • I signed the CLA
  • The title of the PR describes the problem it addresses
  • Each commit's message describes its purpose and effects, and references the issue it resolves
  • If changes are extensive, there is a sequence of easily reviewable commits
  • The code in the PR follows the coding rules
  • There are no unnecessary changes
  • The code compiles and runs on my machine, preferably after each commit individually
  • I created a unit test or vtest to verify the changes I made (if applicable)

@cbjeukendrup
Copy link
Member

So this PR fixes the issue by deleting those default rests during the clone process. But I was wondering: maybe those default rests should never have been created, if the first thing that will happen to them is that they are deleted. I suspect they are created when inserting the staff into the score, so before cloneStaff. Maybe you could look into that.

@skalra4
Copy link
Author

skalra4 commented Apr 18, 2025

I understand that. I have a question: Would you like this to be for every case where a staff member is cloned AND linked? I think the problem is that the functions being used are also what create default staffs, and I'm unsure how to introduce the condition whether or not to remove rests upon default initialization based on the actual time signature. This is my reasoning behind why I did it the way I did. If you have a better suggestion, I can try to implement it, but I can also try my best to do it on my own. However, I did consider this option from your original suggestion, but I couldn't figure out a way to compartmentalize and keep the two scenarios separate.

@cbjeukendrup
Copy link
Member

Score::undoInsertStaff has a createRests parameter, that controls whether default rests are created. It is true by default. Therefore, NotationParts::insertStaff, which is called via NotationParts::doAppendStaff, will always create rests. I think the solution would be to add a createRests parameter to NotationParts::insertStaff and NotationParts::doAppendStaff, and pass that on to undoInsertStaff. Then in NotationParts::appendLinkedStaff, where doAppendStaff is called, pass false to that parameter.

@cbjeukendrup
Copy link
Member

@skalra4 Something went wrong while rebasing. It looks like you first rebased, then pulled the unrebased version from your fork on GitHub, and then pushed. Instead, you should rebase, and then force-push. So:

git pull --rebase upstream master
git push -f

@skalra4 skalra4 force-pushed the 26233-fix_cloneStaff branch from e217c12 to c99db42 Compare April 23, 2025 16:00
@skalra4 skalra4 force-pushed the 26233-fix_cloneStaff branch from c99db42 to 50ea827 Compare April 23, 2025 16:25
@skalra4
Copy link
Author

skalra4 commented Apr 23, 2025

I apologize for the messy working tree. Git still gets me, especially on big projects with many branches and changes.

@cbjeukendrup
Copy link
Member

No worries. It looks correct now!

@skalra4
Copy link
Author

skalra4 commented Apr 23, 2025

Great. Thanks so much for all the help. I appreciate and like the MuseScore dev community a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Combination of pickup measure and linked staff makes score corrupted
2 participants